feat(libuv): polyfill uv_poll, uv_async, uv_default_loop for POSIX#27707
feat(libuv): polyfill uv_poll, uv_async, uv_default_loop for POSIX#27707zobithecat wants to merge 6 commits intooven-sh:mainfrom
Conversation
Implement real uv_poll/uv_async support on Linux (epoll + eventfd) and macOS (kqueue + pipe) so NAPI modules like @serialport/bindings-cpp can work on Bun without modification. Adds: - uv_default_loop() — singleton loop instance - uv_strerror() — 70+ error code mappings - uv_poll_init/start/stop — fd monitoring via epoll/kqueue - uv_async_init/send — cross-thread signaling via eventfd/pipe - uv_close() — handle cleanup for UV_POLL and UV_ASYNC - uv_unref() — no-op (poll thread is detached) - Standalone test (test_uv_polyfill.c) — 17/17 tests pass Updates generate_uv_posix_stubs to exclude polyfilled symbols from the auto-generated stub file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
WalkthroughAdds a POSIX libuv polyfill implementation and tests, exports a new Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts`:
- Around line 2-5: Add a validation step in generate_uv_posix_stubs.ts that
asserts every name listed in polyfilled actually exists in all_symbols before
computing symbols; compute missing = polyfilled.filter(p =>
!all_symbols.includes(p)) and if missing.length > 0 throw a clear error
(including the missing names) so generation fails fast and prevents silent stub
drift between polyfilled and all_symbols; keep the existing symbols =
all_symbols.filter(...) line after this guard.
In `@src/bun.js/bindings/uv-posix-polyfills.c`:
- Around line 159-165: uv_default_loop currently mutates shared singletons
bun__default_loop and bun__default_loop_init without synchronization, causing
races on concurrent first calls; fix by performing one-time thread-safe
initialization (e.g., use pthread_once, a pthread_mutex guard, or C11 atomics)
around the memset so only one thread initializes bun__default_loop and sets
bun__default_loop_init; update the uv_default_loop function to use that chosen
synchronization primitive to protect the memset(&bun__default_loop, 0,
sizeof(uv_loop_t)) and the assignment to bun__default_loop_init so concurrent
callers cannot race.
- Around line 505-520: The bun__ensure_poll_thread function currently does an
unsynchronized check/set on bun__poll_thread_running and ignores pthread_create
failures; make it atomic and error-aware by using a mutex or atomic
compare-and-exchange on bun__poll_thread_running at the start of
bun__ensure_poll_thread to ensure only one initializer proceeds, call
bun__ensure_epoll()/bun__ensure_kqueue() as before, then check the return of
pthread_create when creating bun__poll_thread (with pthread_attr_t usage
unchanged) and if pthread_create fails reset bun__poll_thread_running to 0 and
propagate/log the error so the process doesn't believe the poll thread is
running; reference these symbols: bun__ensure_poll_thread,
bun__poll_thread_running, bun__poll_thread, bun__ensure_epoll,
bun__ensure_kqueue, bun__poll_thread_fn.
- Around line 336-338: The epoll/kevent registration calls (e.g., epoll_ctl
using bun__epoll_fd and bun__poll_wakeup_fd) currently ignore failures and thus
can return a false success; update each registration site (the epoll_ctl and
kevent calls around the wakeup/handle registration) to check the syscall return
value and handle errors: if epoll_ctl/kevent returns -1, capture errno, log or
propagate the error, and return/abort appropriately instead of proceeding as if
registration succeeded; ensure the same change is applied to all analogous calls
(the registrations using bun__epoll_fd, bun__poll_wakeup_fd and the other
kevent/epoll registration sites) so failures cannot be silently ignored.
- Around line 358-390: The poll thread (bun__poll_thread_fn) is invoking
uv_poll_cb, uv_async_cb and uv_close callbacks directly on the detached thread,
violating libuv loop-thread affinity; instead, package callback events
(including uv_events, handle pointer, and a callback-type enum for
POLL/ASYNC/CLOSE) into a thread-safe queue associated with the loop (use
existing bun__poll_entries or a new bun__pending_callbacks list), push the event
while protected by bun__poll_mutex, then signal the loop-thread via the existing
wakeup mechanism (bun__poll_wakeup_fd / bun__poll_drain_wakeup or uv_async_send
equivalent) and return from the poll thread without calling callbacks; implement
loop-side processing to drain that queue and invoke uv_poll_cb, uv_async_cb and
uv_close callbacks on the loop thread (deferring uv_close callbacks instead of
calling them synchronously) so all uv_* callbacks run on the loop thread and
preserve libuv semantics.
In `@test_uv_polyfill.c`:
- Around line 195-198: The test currently hard-codes OS_LINUX and OS_DARWIN via
the OS/OS_LINUX/OS_DARWIN macros, preventing the Darwin path from being
exercised; change the macros so they reflect the build host instead of static
values: update the OS_LINUX and OS_DARWIN definitions to be set conditionally
using compiler predefined macros (e.g. check __APPLE__ / __MACH__ for macOS and
__linux__ for Linux) and leave the OS(x) macro unchanged (refer to OS, OS_LINUX,
OS_DARWIN and the surrounding macro block in test_uv_polyfill.c) so both
platforms are correctly detected at compile time.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/bun.js/bindings/libuv/generate_uv_posix_stubs.tssrc/bun.js/bindings/libuv/generate_uv_posix_stubs_constants.tssrc/bun.js/bindings/uv-posix-polyfills.ctest_uv_polyfill.c
| static void* bun__poll_thread_fn(void* arg) | ||
| { | ||
| (void)arg; | ||
| struct epoll_event events[BUN_UV_MAX_POLL_HANDLES + 1]; | ||
|
|
||
| while (bun__poll_thread_running) { | ||
| int n = epoll_wait(bun__epoll_fd, events, | ||
| BUN_UV_MAX_POLL_HANDLES + 1, 100 /*ms*/); | ||
| if (n < 0) { | ||
| if (errno == EINTR) continue; | ||
| break; | ||
| } | ||
|
|
||
| pthread_mutex_lock(&bun__poll_mutex); | ||
|
|
||
| for (int i = 0; i < n; i++) { | ||
| int fd = events[i].data.fd; | ||
|
|
||
| // Wakeup fd — just drain it | ||
| if (fd == bun__poll_wakeup_fd) { | ||
| bun__poll_drain_wakeup(); | ||
| continue; | ||
| } | ||
|
|
||
| // Find matching poll entry | ||
| for (int j = 0; j < bun__poll_count; j++) { | ||
| if (bun__poll_entries[j].active && bun__poll_entries[j].fd == fd) { | ||
| int uv_events = bun__epoll_to_uv(events[i].events); | ||
| uv_poll_cb cb = bun__poll_entries[j].cb; | ||
| uv_poll_t* handle = bun__poll_entries[j].handle; | ||
| pthread_mutex_unlock(&bun__poll_mutex); | ||
| if (cb) cb(handle, 0, uv_events); | ||
| pthread_mutex_lock(&bun__poll_mutex); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
libuv uv_poll_cb uv_async_cb uv_close callback execution thread loop documentation
💡 Result:
uv_poll_cb(fromuv_poll_start): Runs on the event loop thread as part of the loop’s I/O polling/dispatch (i.e., callbacks for I/O-related handles are invoked when the loop processes I/O events). [1], [2]uv_async_cb(fromuv_async_init): Runs on the event loop thread.uv_async_send()is explicitly safe to call from any thread, and it wakes the loop so the callback executes on the loop thread; sends may be coalesced (multiple sends can result in a single callback). [3], [4]uv_close/uv_close_cb:uv_close()requests an async close; the close callback is deferred and is called by the loop (typically on the next loop iteration). The handle’s memory should only be freed in/after the close callback. The loop design explicitly lists “close callbacks are called” as a step in the loop iteration. [2], [5]- Threading rule of thumb: A
uv_loop_tand its handles are generally not thread-safe to manipulate from other threads; useuv_async_send()to “hop” work onto the loop thread when signaling from other threads. [3], [6]
Sources
[1] libuv uv_poll_t docs (poll handle / uv_poll_cb) — https://docs.libuv.org/en/stable/poll.html
[2] libuv Design overview (loop iteration order: I/O callbacks, then close callbacks, etc.) — https://docs.libuv.org/en/latest/design.html
[3] libuv uv_async_t docs (callback runs on loop thread; uv_async_send thread-safe; coalescing) — https://docs.libuv.org/en/stable/async.html
[4] libuv Guide: Threads (details on async send timing/coalescing guarantees) — https://docs.libuv.org/en/v1.x/guide/threads.html
[5] libuv uv_handle_t / uv_close docs (close callback async; deferred; free in close_cb) — https://docs.libuv.org/en/v1.x/handle.html
[6] libuv uv_loop_t docs (loop functions not thread-safe; general thread-safety warning) — https://docs.libuv.org/en/v1.x/loop.html
🏁 Script executed:
cat -n src/bun.js/bindings/uv-posix-polyfills.c | sed -n '358,395p'Repository: oven-sh/bun
Length of output: 1643
🏁 Script executed:
cat -n src/bun.js/bindings/uv-posix-polyfills.c | sed -n '430,500p'Repository: oven-sh/bun
Length of output: 3148
🏁 Script executed:
cat -n src/bun.js/bindings/uv-posix-polyfills.c | sed -n '745,810p'Repository: oven-sh/bun
Length of output: 2773
Callback execution violates libuv semantics: wrong thread and synchronous close.
Line 389/476 invoke uv_poll_cb on the detached poll thread instead of the loop thread. Line 490 invokes uv_async_cb on the detached poll thread instead of the loop thread. Lines 765/798/803 invoke uv_close callbacks synchronously instead of deferring them. This breaks loop-thread affinity that N-API and JS addons rely on, risking data races, memory corruption, and crashes when addon callbacks interact with JS objects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bun.js/bindings/uv-posix-polyfills.c` around lines 358 - 390, The poll
thread (bun__poll_thread_fn) is invoking uv_poll_cb, uv_async_cb and uv_close
callbacks directly on the detached thread, violating libuv loop-thread affinity;
instead, package callback events (including uv_events, handle pointer, and a
callback-type enum for POLL/ASYNC/CLOSE) into a thread-safe queue associated
with the loop (use existing bun__poll_entries or a new bun__pending_callbacks
list), push the event while protected by bun__poll_mutex, then signal the
loop-thread via the existing wakeup mechanism (bun__poll_wakeup_fd /
bun__poll_drain_wakeup or uv_async_send equivalent) and return from the poll
thread without calling callbacks; implement loop-side processing to drain that
queue and invoke uv_poll_cb, uv_async_cb and uv_close callbacks on the loop
thread (deferring uv_close callbacks instead of calling them synchronously) so
all uv_* callbacks run on the loop thread and preserve libuv semantics.
- uv_default_loop: use pthread_once for thread-safe singleton init - bun__ensure_poll_thread: use pthread_once, check pthread_create errors - epoll_ctl/kevent: check return values and propagate errors at all registration sites (wakeup fd, async init, poll start) - generate_uv_posix_stubs.ts: validate polyfilled symbols exist in all_symbols to prevent silent stub drift - test_uv_polyfill.c: detect OS at compile time via __linux__/__APPLE__ - Add design note explaining poll-thread callback dispatch rationale Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/bun.js/bindings/uv-posix-polyfills.c (2)
527-533:⚠️ Potential issue | 🟠 MajorPoll-thread startup failures are not propagated, so APIs can return false success.
If
pthread_createfails (Line [530]-Line [533]), the code only logs; thenpthread_once(Line [538]) prevents retries.uv_poll_start(Line [587]) anduv_async_init(Line [696]) continue without surfacing initialization failure.🛠️ Suggested fix
static pthread_once_t bun__poll_thread_once = PTHREAD_ONCE_INIT; +static int bun__poll_thread_init_err = 0; static void bun__poll_thread_init(void) { @@ int err = pthread_create(&bun__poll_thread, &attr, bun__poll_thread_fn, NULL); @@ if (err != 0) { bun__poll_thread_running = 0; + bun__poll_thread_init_err = UV__ERR(err); fprintf(stderr, "bun: failed to create poll thread: %s\n", strerror(err)); } } -static void bun__ensure_poll_thread(void) +static int bun__ensure_poll_thread(void) { - pthread_once(&bun__poll_thread_once, bun__poll_thread_init); + int once_err = pthread_once(&bun__poll_thread_once, bun__poll_thread_init); + if (once_err != 0) return UV__ERR(once_err); + if (!bun__poll_thread_running) return bun__poll_thread_init_err != 0 ? bun__poll_thread_init_err : UV_EIO; + return 0; } @@ - bun__ensure_poll_thread(); + int thread_err = bun__ensure_poll_thread(); + if (thread_err != 0) return thread_err; @@ - bun__ensure_poll_thread(); + int thread_err = bun__ensure_poll_thread(); + if (thread_err != 0) return thread_err;Also applies to: 536-539, 587-587, 696-696
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/bindings/uv-posix-polyfills.c` around lines 527 - 533, pthread_create failures are only logged and not propagated, so subsequent initialization APIs (pthread_once seed, uv_poll_start, uv_async_init) can incorrectly succeed; modify the poll-thread bootstrap (the block that calls pthread_create for bun__poll_thread and uses pthread_once with bun__poll_thread_fn) to record the creation failure in a persistent status (e.g., set bun__poll_thread_running = 0 and set a new bun__poll_thread_error flag/code) and have callers like uv_poll_start and uv_async_init check that status and return an error instead of proceeding; ensure the pthread_once-invoked initializer (bun__poll_thread_fn) sets the error status on pthread_create failure so retries or other APIs see the failed state and propagate/return failure to callers.
262-270:⚠️ Potential issue | 🔴 Critical
uv_poll_cb/uv_async_cb/uv_close_cbexecution model is incompatible with libuv’s callback contract.Line [262]-Line [270] documents poll-thread callback dispatch, and callbacks are invoked at Line [396], Line [410], Line [483], and Line [497];
uv_closecallbacks are called synchronously at Line [813] and Line [846]. This breaks loop-thread affinity/deferred-close semantics expected by many native addons and can introduce race/crash behavior.In libuv v1.x documentation, on which thread must `uv_poll_cb` and `uv_async_cb` run, and is `uv_close_cb` deferred to a later loop iteration?Also applies to: 395-410, 483-498, 813-847
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/bindings/uv-posix-polyfills.c` around lines 262 - 270, The current code directly invokes uv_poll_cb/uv_async_cb/uv_close_cb on the poll thread which violates libuv’s requirement that these callbacks run on the loop (dispatch) thread and that close callbacks be deferred to a later loop iteration; change the implementation so poll and async events enqueue a small task/closure onto the loop’s pending callback queue (or use the existing loop-run-phase dispatch mechanism) and have the loop thread drain and call the registered uv_poll_cb and uv_async_cb there, and for uv_close schedule the uv_close_cb to be invoked on the next loop iteration rather than synchronously; reference the uv_poll_cb, uv_async_cb, uv_close_cb dispatch sites and the pending-queue/dispatch code path that currently invokes callbacks on the poll thread and replace direct calls with enqueuing and loop-thread dispatcher invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/uv-posix-polyfills.c`:
- Around line 670-677: The delete/unregister calls to epoll_ctl and kevent
(EPOLL_CTL_DEL / EV_DELETE) in the unregister path are ignoring return values;
update the code that calls epoll_ctl(bun__epoll_fd, EPOLL_CTL_DEL, ...) and
kevent(bun__kqueue_fd, kev, 2, ...) (and other EV_DELETE sites handling
bun__poll_entries[]) to check the return value and errno like the corresponding
EPOLL_CTL_ADD/EPOLL_CTL_MOD and EV_ADD sites do, and handle/log errors
consistently (e.g., use the same error logging or handling function used
elsewhere in this file so failures to remove a FD are not silently ignored).
In `@test_uv_polyfill.c`:
- Around line 297-307: The tests currently continue after initialization
failures causing use of uninitialized structures; modify the branches after
uv_poll_init (the uv_poll_init call that sets 'handle'), uv_poll_start (the
uv_poll_start call using 'handle'), and uv_async_init (the uv_async_init that
sets another 'handle') to detect non-zero returns, perform the same cleanup used
after pipe() failures (close any opened fds and stop/close loop if appropriate)
and immediately return or goto a cleanup label so subsequent checks (e.g.,
reading handle.type) are not executed on an invalid handle; also check the
return value of pthread_create (the variable 'tid') and on failure skip
pthread_join and perform cleanup instead. Ensure you reference and protect the
same 'handle' variables and the pthread 'tid' when applying these early-return
cleanup paths.
- Around line 268-276: Replace the non-thread-safe volatile flags with C11
atomics: change the declarations of poll_callback_called and
poll_callback_events from "static volatile int" to "static _Atomic int" and
update all accesses (writes in test_poll_cb and reads in the main test thread)
to use atomic_store_explicit/atomic_load_explicit with appropriate ordering
(e.g., atomic_store_explicit(..., memory_order_release) in test_poll_cb and
atomic_load_explicit(..., memory_order_acquire) where the main thread reads
them) to ensure correct synchronization between the libuv callback
(test_poll_cb) and the test thread.
---
Duplicate comments:
In `@src/bun.js/bindings/uv-posix-polyfills.c`:
- Around line 527-533: pthread_create failures are only logged and not
propagated, so subsequent initialization APIs (pthread_once seed, uv_poll_start,
uv_async_init) can incorrectly succeed; modify the poll-thread bootstrap (the
block that calls pthread_create for bun__poll_thread and uses pthread_once with
bun__poll_thread_fn) to record the creation failure in a persistent status
(e.g., set bun__poll_thread_running = 0 and set a new bun__poll_thread_error
flag/code) and have callers like uv_poll_start and uv_async_init check that
status and return an error instead of proceeding; ensure the
pthread_once-invoked initializer (bun__poll_thread_fn) sets the error status on
pthread_create failure so retries or other APIs see the failed state and
propagate/return failure to callers.
- Around line 262-270: The current code directly invokes
uv_poll_cb/uv_async_cb/uv_close_cb on the poll thread which violates libuv’s
requirement that these callbacks run on the loop (dispatch) thread and that
close callbacks be deferred to a later loop iteration; change the implementation
so poll and async events enqueue a small task/closure onto the loop’s pending
callback queue (or use the existing loop-run-phase dispatch mechanism) and have
the loop thread drain and call the registered uv_poll_cb and uv_async_cb there,
and for uv_close schedule the uv_close_cb to be invoked on the next loop
iteration rather than synchronously; reference the uv_poll_cb, uv_async_cb,
uv_close_cb dispatch sites and the pending-queue/dispatch code path that
currently invokes callbacks on the poll thread and replace direct calls with
enqueuing and loop-thread dispatcher invocation.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/bun.js/bindings/libuv/generate_uv_posix_stubs.tssrc/bun.js/bindings/uv-posix-polyfills.ctest_uv_polyfill.c
| static volatile int poll_callback_called = 0; | ||
| static volatile int poll_callback_events = 0; | ||
|
|
||
| void test_poll_cb(uv_poll_t* handle, int status, int events) { | ||
| (void)handle; | ||
| (void)status; | ||
| poll_callback_called = 1; | ||
| poll_callback_events = events; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
C11 volatile vs _Atomic stdatomic.h inter-thread synchronization memory safety
💡 Result:
C11 volatile vs C11 _Atomic (<stdatomic.h>) for inter-thread synchronization & memory safety
volatile (C)
- What it does: Prevents certain compiler optimizations for that object (each volatile access is an “observable side effect” in the abstract machine). This is a single-thread (as-if) guarantee. [1]
- What it does not do (for threads):
- No atomicity (a concurrent read/write can still be a data race). [1]
- No inter-thread synchronization / happens-before (other threads are not required to observe volatile accesses in that order). [1][4]
- No ordering of other memory (non-volatile operations may be reordered around volatile accesses). [4]
- Consequence: Using
volatileas a “thread-shared flag” is not memory-safe in C11; it can still be undefined behavior due to data races. [1]
_Atomic + <stdatomic.h> (C11)
- What it provides: Atomic objects/operations designed for concurrency; atomic objects are the ones intended to be free from data races when accessed concurrently. [3]
- Synchronization & visibility: Many atomic operations can be given memory ordering (e.g.,
memory_order_release/memory_order_acquire) to create inter-thread synchronization and make prior writes visible across threads (i.e., establish happens-before via the C memory model’s synchronization rules). [3][4] - APIs: Use
atomic_load(_explicit),atomic_store(_explicit), etc.; defaultatomic_storeis sequentially consistent, andatomic_store_explicitsupportsrelaxed/release/seq_cst(wrong orders are UB). [5]
Common guidance
- If multiple threads access the same object and at least one access is a write, use
_Atomicoperations or a mutex/condvar—notvolatile—to avoid data races (UB) and to get required visibility/order guarantees. [1][3][4]
Sources
[1] cppreference (C) — volatile qualifier: https://cppreference.net/c/language/volatile.html
[2] cppreference (C) — <stdatomic.h> overview: https://en.cppreference.com/w/c/header/stdatomic.html
[3] cppreference (C) — atomic types (_Atomic): https://cppreference.net/c/language/atomic.html
[4] cppreference (C) — memory_order + relationship with volatile: https://cppreference.net/c/atomic/memory_order.html
[5] cppreference (C) — atomic_store(_explicit): https://cppreference.com/w/c/atomic/atomic_store.html
🏁 Script executed:
# First, locate the test_uv_polyfill.c file
find . -name "test_uv_polyfill.c" -type fRepository: oven-sh/bun
Length of output: 75
🏁 Script executed:
# Examine the file at the specified line ranges
head -n 380 ./test_uv_polyfill.c | tail -n +260 | cat -nRepository: oven-sh/bun
Length of output: 4319
Use atomics for callback flags; volatile is not thread-safe synchronization in C.
Lines 268–269, 278, and 339 are written from callback threads (executed in the libuv event loop) and read from the main test thread. The C11 standard does not provide inter-thread synchronization or visibility guarantees for volatile; this is a data race and undefined behavior.
Use _Atomic with appropriate memory ordering instead:
🛠️ Suggested fix
+#include <stdatomic.h>
@@
-static volatile int poll_callback_called = 0;
-static volatile int poll_callback_events = 0;
+static _Atomic int poll_callback_called = 0;
+static _Atomic int poll_callback_events = 0;
@@
- poll_callback_called = 1;
- poll_callback_events = events;
+ atomic_store_explicit(&poll_callback_events, events, memory_order_relaxed);
+ atomic_store_explicit(&poll_callback_called, 1, memory_order_release);
@@
-static volatile int close_cb_called = 0;
+static _Atomic int close_cb_called = 0;
@@
- close_cb_called = 1;
+ atomic_store_explicit(&close_cb_called, 1, memory_order_release);
@@
-static volatile int async_callback_called = 0;
+static _Atomic int async_callback_called = 0;
@@
- async_callback_called = 1;
+ atomic_store_explicit(&async_callback_called, 1, memory_order_release);Also applies to: 278–282, 315–318, 339–344, 372–376
🧰 Tools
🪛 Cppcheck (2.19.0)
[style] 271-271: The function 'test_poll_cb' should have static linkage since it is not used outside of its translation unit.
(staticFunction)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test_uv_polyfill.c` around lines 268 - 276, Replace the non-thread-safe
volatile flags with C11 atomics: change the declarations of poll_callback_called
and poll_callback_events from "static volatile int" to "static _Atomic int" and
update all accesses (writes in test_poll_cb and reads in the main test thread)
to use atomic_store_explicit/atomic_load_explicit with appropriate ordering
(e.g., atomic_store_explicit(..., memory_order_release) in test_poll_cb and
atomic_load_explicit(..., memory_order_acquire) where the main thread reads
them) to ensure correct synchronization between the libuv callback
(test_poll_cb) and the test thread.
test_uv_polyfill.c
Outdated
| TEST("uv_poll_init returns 0"); | ||
| int r = uv_poll_init(loop, &handle, pipefd[0]); | ||
| if (r == 0) PASS(); else FAIL("non-zero return"); | ||
|
|
||
| TEST("handle type is UV_POLL"); | ||
| if (handle.type == UV_POLL) PASS(); else FAIL("wrong type"); | ||
|
|
||
| TEST("uv_poll_start returns 0"); | ||
| poll_callback_called = 0; | ||
| r = uv_poll_start(&handle, UV_READABLE, test_poll_cb); | ||
| if (r == 0) PASS(); else FAIL("non-zero return"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "test_uv_polyfill.c" -type fRepository: oven-sh/bun
Length of output: 75
🏁 Script executed:
wc -l ./test_uv_polyfill.cRepository: oven-sh/bun
Length of output: 79
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '290,320p'Repository: oven-sh/bun
Length of output: 1217
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '355,380p'Repository: oven-sh/bun
Length of output: 1100
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '280,295p'Repository: oven-sh/bun
Length of output: 455
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '350,365p'Repository: oven-sh/bun
Length of output: 559
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '1,50p'Repository: oven-sh/bun
Length of output: 1641
🏁 Script executed:
rg -n "define PASS|define FAIL|define TEST" ./test_uv_polyfill.cRepository: oven-sh/bun
Length of output: 139
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '216,235p'Repository: oven-sh/bun
Length of output: 604
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '320,350p'Repository: oven-sh/bun
Length of output: 1136
🏁 Script executed:
cat -n ./test_uv_polyfill.c | sed -n '375,402p'Repository: oven-sh/bun
Length of output: 1203
Abort test branches on setup failure to avoid undefined behavior and invalid-state execution.
Setup failures do not halt test execution:
- After failed
uv_poll_init/uv_poll_start(lines 299, 307), code continues and accesses uninitializedhandle.type(line 302) and calls operations on uninitialized handle. - After failed
uv_async_init(line 361), code continues and accesses uninitializedhandle.type(line 364). pthread_createreturn value is ignored (line 369); if it fails,pthread_join(line 375) is called with an uninitializedtid, causing undefined behavior.
Add early returns with cleanup on initialization failure, following the pattern already used for pipe() (lines 289–291).
🛠️ Suggested fix pattern
int r = uv_poll_init(loop, &handle, pipefd[0]);
- if (r == 0) PASS(); else FAIL("non-zero return");
+ if (r == 0) PASS(); else { FAIL("non-zero return"); close(pipefd[0]); close(pipefd[1]); return; }
@@
r = uv_poll_start(&handle, UV_READABLE, test_poll_cb);
- if (r == 0) PASS(); else FAIL("non-zero return");
+ if (r == 0) PASS(); else { FAIL("non-zero return"); uv_close((uv_handle_t*)&handle, NULL); close(pipefd[0]); close(pipefd[1]); return; }
@@
int r = uv_async_init(loop, &handle, test_async_cb);
- if (r == 0) PASS(); else FAIL("non-zero return");
+ if (r == 0) PASS(); else { FAIL("non-zero return"); return; }
@@
- pthread_create(&tid, NULL, async_sender_thread, &handle);
+ if (pthread_create(&tid, NULL, async_sender_thread, &handle) != 0) {
+ FAIL("pthread_create failed");
+ uv_close((uv_handle_t*)&handle, NULL);
+ return;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test_uv_polyfill.c` around lines 297 - 307, The tests currently continue
after initialization failures causing use of uninitialized structures; modify
the branches after uv_poll_init (the uv_poll_init call that sets 'handle'),
uv_poll_start (the uv_poll_start call using 'handle'), and uv_async_init (the
uv_async_init that sets another 'handle') to detect non-zero returns, perform
the same cleanup used after pipe() failures (close any opened fds and stop/close
loop if appropriate) and immediately return or goto a cleanup label so
subsequent checks (e.g., reading handle.type) are not executed on an invalid
handle; also check the return value of pthread_create (the variable 'tid') and
on failure skip pthread_join and perform cleanup instead. Ensure you reference
and protect the same 'handle' variables and the pthread 'tid' when applying
these early-return cleanup paths.
Abort test branches on setup failure to avoid operating on uninitialized handles. Check pthread_create return value to prevent pthread_join on invalid tid. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- bun__ensure_poll_thread now returns int error code; callers (uv_poll_start, uv_async_init) check and propagate failure - Store bun__poll_thread_init_err so pthread_once does not mask pthread_create failures on subsequent calls - Log EPOLL_CTL_DEL failures in uv_poll_stop and uv_close (ignoring EBADF/ENOENT which are expected during cleanup) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/bun.js/bindings/uv-posix-polyfills.c (2)
685-687:⚠️ Potential issue | 🟠 Major
kevent(... EV_DELETE ...)failures are still ignored on macOS cleanup paths.Line 686 and Line 848 drop delete errors completely. Unexpected failures can leave stale registrations behind; Linux delete paths already log non-
ENOENT/EBADFerrors.Consider matching Linux behavior here: ignore expected not-found/bad-fd cases, but log other errors.
Also applies to: 847-849
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun.js/bindings/uv-posix-polyfills.c` around lines 685 - 687, The kevent calls that attempt EV_DELETE (using kevent with bun__kqueue_fd and kev) currently ignore errors; update both delete paths that call kevent(bun__kqueue_fd, kev, 2, NULL, 0, NULL) to check the return value and errno, silently ignore expected errno values (ENOENT and EBADF) but log any other failures with context (operation, the fd or identifier being deleted and errno/strerror) so unexpected errors are not dropped; apply the same change to both cleanup sites where EV_DELETE is used.
257-270:⚠️ Potential issue | 🔴 CriticalCallback/close execution model can race into UAF and still diverges from libuv lifecycle semantics.
Line 396/Line 410/Line 483/Line 497 call user callbacks on the detached poll thread, while Line 823/Line 859 invoke
close_cbsynchronously. With the lock dropped around callback execution, another thread can close and free the same handle before/in parallel with callback use (notably,src/bun.js/bindings/ProcessBindingTTYWrap.cppLine 40-Line 49 deletes inuv_closecallback). This is a memory-safety risk.Please serialize callback and close-callback execution for a handle and defer
close_cbuntil no in-flight callback can still touch the handle.In libuv v1.x, are uv_poll_cb/uv_async_cb required to run on the loop thread, and is uv_close callback deferred asynchronously rather than invoked synchronously?Also applies to: 396-410, 483-498, 803-865
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bun.js/bindings/uv-posix-polyfills.c`:
- Around line 610-656: The macOS path currently only adds filters and never
removes ones that were previously registered, so when a handle drops interest
(e.g., from UV_READABLE|UV_WRITABLE to UV_READABLE) the EVFILT_WRITE stays
active; to fix, capture the previous interest bits before overwriting (save
prev_events = entry->events), then build kev[] entries to EV_DELETE any filter
where (prev_events & FILTER) && !(events & FILTER) and EV_ADD|EV_ENABLE for
newly requested filters where !(prev_events & FILTER) && (events & FILTER); call
kevent(bun__kqueue_fd, kev, nkev, NULL, 0, NULL) if nkev>0 and handle errors the
same way as the existing kevent error path (set entry->active=0,
pthread_mutex_unlock(&bun__poll_mutex), return UV__ERR(saved_errno)); use the
same symbols kev, EV_DELETE, EV_ADD, EV_ENABLE, bun__kqueue_fd, UV_READABLE,
UV_WRITABLE, entry->events to locate and modify the code.
---
Duplicate comments:
In `@src/bun.js/bindings/uv-posix-polyfills.c`:
- Around line 685-687: The kevent calls that attempt EV_DELETE (using kevent
with bun__kqueue_fd and kev) currently ignore errors; update both delete paths
that call kevent(bun__kqueue_fd, kev, 2, NULL, 0, NULL) to check the return
value and errno, silently ignore expected errno values (ENOENT and EBADF) but
log any other failures with context (operation, the fd or identifier being
deleted and errno/strerror) so unexpected errors are not dropped; apply the same
change to both cleanup sites where EV_DELETE is used.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/bun.js/bindings/uv-posix-polyfills.c
…rrors - uv_poll_start: on macOS, EV_DELETE previously registered filters (EVFILT_READ/EVFILT_WRITE) when they are dropped from the new interest set, preventing stale events from firing - kevent EV_DELETE: log unexpected errors (skip ENOENT/EBADF) in uv_poll_stop and uv_close, matching Linux epoll_ctl DEL behavior - Guard prev_events with #if defined(__APPLE__) to suppress -Wunused-variable on Linux Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/bun.js/bindings/uv-posix-polyfills.c (1)
395-411:⚠️ Potential issue | 🔴 CriticalCallback/close execution model can still violate libuv handle lifetime guarantees.
uv_poll_cb/uv_async_cbare invoked on the detached poll thread, whileuv_closeinvokesclose_cbsynchronously. This combination can race with concurrent close paths and trigger callback-on-freed-handle behavior (UAF risk), and it diverges from libuv’s loop-thread/deferred-close contract relied on by addons.In libuv v1.x documentation, are `uv_poll_cb` and `uv_async_cb` guaranteed to run on the event-loop thread, and is `uv_close` callback execution deferred to a later loop iteration rather than synchronous?Also applies to: 482-499, 839-840, 876-877
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/bun.js/bindings/uv-posix-polyfills.c
2 participants
Summary
Implements real
uv_poll/uv_asyncsupport on POSIX so that NAPI modules relying on libuv's I/O polling (e.g.@serialport/bindings-cpp) work on Bun without modification.What's added
uv_default_loop()uv_loop_tinstance for NAPI modulesuv_strerror()uv_poll_init/uv_poll_init_socketuv_poll_start/uv_poll_stopuv_async_init/uv_async_senduv_close()UV_POLLandUV_ASYNCtypesuv_unref()Architecture
epoll_wait(Linux) orkevent(macOS) to monitor all registered fdspthread_mutex; callbacks dispatched on the poll threadeventfd(Linux) /pipe(macOS) to signal the poll thread on registration changesFiles changed
src/bun.js/bindings/uv-posix-polyfills.c— +675 lines: poll thread, uv_poll, uv_async, uv_close, uv_strerror, uv_default_loopsrc/bun.js/bindings/libuv/generate_uv_posix_stubs_constants.ts— Addedpolyfilledarray (10 symbols)src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts— Filter polyfilled symbols from stub generationtest_uv_polyfill.c— Standalone test harness (17/17 tests pass)Motivation
Many popular NAPI modules (serial ports, USB, GPIO) depend on
uv_pollto watch file descriptors. Currently Bun stubs these functions with__bun_throw_not_implemented, making these modules unusable. This PR provides a minimal but functional implementation that unblocks the NAPI ecosystem.Test plan
test_uv_polyfill.c) covers all new functions — 17/17 pass@serialport/bindings-cppNAPI module🤖 Generated with Claude Code